Content reports ported from DSpace 6.x #2163
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/app/admin/admin-reports/filtered-collections/filtered-collections.component.html
Fixed
Show fixed
Hide fixed
src/app/admin/admin-reports/filtered-collections/filtered-collections.component.html
Fixed
Show fixed
Hide fixed
tdonohue
added
new feature
component: administrative tools
|
Hi @jeffmorin, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
I resolved the existing conflicts and re-updated my fork (and this PR). All of them were in i18n files. |
|
@jeffmorin : If it's easier to maintain this PR, you are welcome to remove all the changes to i18n files except the ones in |
|
Thanks for the info, I'll happily keep it in mind for future updates! |
|
About the bug in the Metadata Query report: First of all, I cannot reproduce the behaviour you are describing, I navigate between pages without any gap in the numbering. Besides, you wrote “with no filters (or anything that returns several pages of results)” — how can you navigate between pages without setting anything that returns several pages of results? I am not sure I understand what you mean: did you get paginated results or not? |
|
I couldn't reproduce the erratic behaviour in the Browse by Title and Browse by Publication Date functionalities. |
|
I finally reproduced the results numbering bug (1–10 then 101–110, and so on). An important repro step I was missing is that I must query a repository containing more than 100 items. I will check this too. |
|
The navigation bug is fixed. It was an inconsistency in parameter naming that I didn't see when switching to GET requests. |
|
@artlowel : If you have a moment this week, could you see if you are able to reproduce the issue with Browse by Title / Date that I've run into with this PR? I've found that pulling this PR onto latest Basically, I'm able to reliably reproduce this bug anytime I test these PRs (both yesterday and today). But, @jeffmorin isn't able to reproduce it. If you or someone on your team has time to just test whether you see it, that might be helpful here. |
|
Hi @jeffmorin, |
|
@tdonohue I can't reproduce those browse issues. I tried while logged out, and logged in (having first used the content reports) for both the global and collection browse lists, all seem to work. Perhaps it's something specific to (one of) the items returned by those pages on your backend? Does it work if you go straight to a different page? |
|
Hi @artlowel. Which version of the code did you use to try to reproduce the bug Tim ran into? Yesterday, I refactored the |
|
@jeffmorin I used the latest version |
| OptionVO.item('50', '50'), | ||
| OptionVO.item('100', '100'), | ||
| OptionVO.item('250', '250'), | ||
| OptionVO.item('1000', '1000') |
There was a problem hiding this comment.
@jeffmorin : Could we consider removing this 1,000 option (and perhaps 250 as well). I mistakenly tried using it and my entire system froze while the backend attempted to respond with 1,000 items.
If possible, I'd like us to consider placing limits on these page sizes to 100 or less. It'd also be ideal to have the DEFAULT value be simply 10. (It appears the current default is 100...which also responds very slowly for me.)
There was a problem hiding this comment.
👍 Thanks @jeffmorin ! This frontend is now working for me and looks good. That said, I had a very minor comment that I submitted just a few minutes ago: https://github.com/DSpace/dspace-angular/pull/2163/files#r1506309146
I did notice the the pagesize options include a 1,000. I mistakenly tried that, and my system hung for several minutes. I think we should consider removing anything under pagesize 100 and set the default to 10. However, I'll leave this as optional based on your feedback.
|
@jeffmorin : I've decided to merge this as-is just to get it onto |
|
Flagging this as |
tdonohue
added
the
needs documentation
|
Documentation for this feature exists in https://wiki.lyrasis.org/pages/viewpage.action?pageId=325255348 Removing the "needs documentation" flag. |
tdonohue
removed
the
needs documentation
References
Description
This pull request includes an initial version of the two content reports ported from DSpace 6.x.
Instructions for Reviewers
This pull request adds a Content Reports section in the Administration menu to provide access to both reports from the Angular web application.
After logging in using an account with administrator permissions, select the Report item from the Administration menu at the left of the page, then select one of the two entries in this section.
In the Filtered Items report, the Export Metadata functionality is not yet implemented, it will be part of another pull request, once this one is all settled and merged.
Please note:
filtered-collections-component.tsandfiltered-items-component.ts), I use a DSpaceRestService instance to send my requests to the REST layer. It may be better to create and use a specific data service similar toCollectionDataService, but I will need help to do so if requested.filtered-items-component.spec.tsrun properly. I will definitely need help with this.Checklist
yarn run lint[Not sure]package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.